ci: use builder on windows platforms#1961
Conversation
Clippy Allow Annotation ReportComparing clippy allow annotations between branches:
Summary by Rule
Annotation Counts by File
Annotation Stats by Crate
About This ReportThis report tracks Clippy allow annotations for specific rules, showing how they've changed in this PR. Decreasing the number of these annotations generally improves code quality. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1961 +/- ##
==========================================
- Coverage 72.87% 72.83% -0.05%
==========================================
Files 457 458 +1
Lines 75769 75789 +20
==========================================
- Hits 55220 55204 -16
- Misses 20549 20585 +36
🚀 New features to boost your workflow:
|
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: d383f31 | Docs | Datadog PR Page | Give us feedback! |
9c30399 to
f3b0432
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dfeffd0b47
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
4e141e3 to
8a4955f
Compare
gleocadie
left a comment
There was a problem hiding this comment.
overall looks good. The other PR has a comment about enabled features
Artifact Size Benchmark Reportaarch64-alpine-linux-musl
aarch64-unknown-linux-gnu
libdatadog-x64-windows
libdatadog-x86-windows
x86_64-alpine-linux-musl
x86_64-unknown-linux-gnu
|
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
Tests failed on this commit 0a02f45: What to do next?
|
|
/merge -c |
|
View all feedbacks in Devflow UI.
This merge request is not in the queue and can't be unqueued To get help about command usage, write If you need support, contact us on Slack #devflow with those details! |
|
/merge |
|
View all feedbacks in Devflow UI.
This pull request is not mergeable according to GitHub. Common reasons include pending required checks, missing approvals, or merge conflicts — but it could also be blocked by other repository rules or settings.
The expected merge time in
|
What does this PR do?
Switches the Windows release-artifact build over to the shared
buildercrate, so all platforms (Linux/glibc, Linux/musl, macOS, Windows) now go through the same path. To make that possible:builder/src/arch/windows.rs: adds Windows support to the builder — copies the PDB and the*.dll.libimport library, uses the_ffi-suffixed artifact names produced by the FFI crate, and no-ops the pkg-config step (Windows has nopkg-config).builder/src/arch/mod.rs+profiling.rs(refactor: move add_pkg_config to arch): hoists the per-platformadd_pkg_configinto eacharch/*.rs, with the shared file-templating logic exposed asarch::generate_pkg_config. This iswhat lets Windows opt out cleanly instead of carrying a
#[cfg]inprofiling.rs.builder/src/builder.rs: skips creatingbin/andpkgconfig/directories on Windows, where they're unused.windows/build-artifacts.ps1: deleted — its responsibilities now live in thebuilder.cargo nextest runinvocations now pass--no-tests=passso the workflow doesn't fail when a partition/affected-crate set yields zero tests..github/workflows/test.ymlno longer hand-filtersbuilder/test_spawn_from_libout of the package list —--no-tests=passmakes that filtering unnecessary, and dropping it means thebuildercrate's own tests are actuallyexercised by CI when it changes.
miri.ymlandcoverage.yml.Motivation
We've been maintaining two parallel build pipelines: every other platform goes through the
buildercrate, while Windows had its own PowerShell script (windows/build-artifacts.ps1) that re-implemented overlapping logic (artifact naming,output layout, header post-processing). Any change to how artifacts are produced had to be made (and kept in sync) in two places. Routing Windows through the
builderconsolidates this and lets future build changes ship in one place.The CI changes are required collateral: once
builderis the path used for the Windows release build, the workflow's hard-coded--exclude builderis wrong, and partition-style invocations need--no-tests=passso the workflow toleratesempty test sets.
Additional Notes
libddprof-build: this PR depends on a matching branch in the privatelibddprof-buildrepo. The last commit (dfeffd0b4 chore: reference libddprof branch that matches the changes in the builder) temporarily points.gitlab-ci.yml'sDOWNSTREAM_BRANCHat that branch. This commit must be reverted tomainbefore merge — please flag this in review if I forget.datadog_profiling.{dll,lib,pdb}todatadog_profiling_ffi.{dll,lib,pdb}plus the newdatadog_profiling_ffi.dll.libimport library. Downstream consumers that hard-coded the old names will need to follow.